-
Notifications
You must be signed in to change notification settings - Fork 263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Predicate Pushdown for video table #273
Conversation
gaurav274
commented
Jul 27, 2022
- Push down the predicate on the video table to avoid decoding all the frames.
- Supports simple range-based and equality-based predicates.
statement_binder_context.py
# System only supports pushing basic range predicates on id | ||
video_alias = lget.video.alias | ||
col_alias = f"{video_alias}.id" | ||
pushdown_pred, unsupported_pred = extract_pushdown_predicate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we try avoiding doing the same computation in rule check and apply?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought of it. But, the only way to avoid it is by keeping the state across check and apply which might creep in bugs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, do you have any thoughts on it? I might be doing the same in future rules as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can either keep the state in rule or in optimizer context. I think both we need to clean up afterwards. To enforce the cleaning up, we may can implement a step in the base class, though not sure it is a good idea or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the optimizer context
idea. But it is too much engineering for now. I do not think the computation is expensive enough for us to spend time on this optimization.
RuntimeError: Invalid expression | ||
|
||
Example: | ||
extract_range_from_comparison_expr(id < 10, 0, inf): True, [(0,9)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we input inf
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inf is just an example. For actual code, we would always know the upper and lower bound. The ultimate goal is to get it from catalog and pass it down the pipeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM